Explicit namespaces in sort-related docstrings #53968
Conversation
jishnub
added
docs
There was a problem hiding this comment.
These seem like improvements
Changing CountingSort <: Algorithm to CountingSort() isa Algorithm would not be an improvement on its own, as the declaration of Counting sort is literally struct CountingSort <: Algorithm end, but I guess it makes sense for consistency.
This also documents the internals from a Base user's perspective which, absent the public keyword, would be a problem, but with that it's clear these are not public so it's okay to document them in a way that focuses on the experience of Base users poking around.
Overall, LGTM. Thanks!
Co-authored-by: Lilith Orion Hafner <[email protected]>
Co-authored-by: Lilith Orion Hafner <[email protected]>
|
Personally I don't prefer the |
This represents the unexported names by the fully qualified namespace, which makes it simpler to interpret the keyword arguments. Otherwise, for example, it's not obvious where
Forwardcomes from in thesortdocstring.Also, replaces some occurrences of
instance <: Algorithmbyinstance isa Algorithm, which is the correct relation. This is also closer to English, which helps with readability.Additionally, I think
Base.Sort.defalgshould be documented, as this is being used in the function signatures. This may be done separately.